Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add TLS support for diesel-async database connections #3189

Merged
merged 1 commit into from
Jun 26, 2023
Merged

Add TLS support for diesel-async database connections #3189

merged 1 commit into from
Jun 26, 2023

Conversation

sunaurus
Copy link
Collaborator

@sunaurus sunaurus commented Jun 18, 2023 •
edited
Loading

This PR adds a new tls_enabled configuration option in the database block of lemmy.hsjon. When configured to true, Lemmy will create SSL connections to the postgres database through diesel-async.

By default, the database certificate will be verified using the OS certificate store (thanks to rustls-native-certs), but users can also provide a path to a CA certificate file directly using the LEMMY_DATABASE_CERT_PATH env variable if necessary.

Note, by default tls_enabled will be false, so using SSL connections will be entirely opt-in.

This fixes #3007

@sunaurus sunaurus marked this pull request as draft June 18, 2023 21:24
@sunaurus sunaurus marked this pull request as ready for review June 18, 2023 21:50
dessalines
dessalines reviewed Jun 19, 2023
View reviewed changes
crates/utils/src/settings/structs.rs Outdated Show resolved Hide resolved
crates/db_schema/src/utils.rs Outdated Show resolved Hide resolved
crates/db_schema/src/utils.rs Outdated Show resolved Hide resolved
@sunaurus
Copy link
Collaborator Author

sunaurus commented Jun 23, 2023 •
edited
Loading

I have significantly reduced the scope of this PR: it only aims to provide support for sslmode=require now (which means that a TLS connection will be used - preventing eavesdropping - but the server identity will not be verified).

No new configuration parameters are needed anymore, sslmode=require is parsed from the database connection string.

After discussions with some other admins, I realized that sslmode=require support would provide the biggest ROI while keeping the code much simpler compared to trying to support additional sslmodes.

@sunaurus sunaurus requested a review from dessalines June 23, 2023 18:27
@Nutomic Nutomic merged commit 6d67f88 into LemmyNet:main Jun 26, 2023
@sunaurus sunaurus deleted the sslmode branch June 26, 2023 08:40
Nutomic pushed a commit that referenced this pull request Jun 26, 2023
@sunaurus @Nutomic
Add support for sslmode=require for diesel-async DB connections (#3189)
6b28f8c
@tristanisham
Copy link

I see what you did here 👀

ruffsl added a commit to ruffsl/lemmy-docs that referenced this pull request Jul 14, 2023
@ruffsl
Update info on tls_enabled in from_scratch.md
68c1054 
Update docs to reflect changes from LemmyNet/lemmy#3189
@ruffsl ruffsl mentioned this pull request Jul 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nutomic Nutomic Nutomic approved these changes

@dessalines dessalines Awaiting requested review from dessalines dessalines is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Connecting to a database with sslmode=require fails
4 participants
@sunaurus @tristanisham @dessalines @Nutomic